Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

kinke · 2020-05-07T00:43:26Z

The test adaptations are mostly a revert of 3893840.

@weak global variables are still left out but could be emulated exactly the same way.

kinke · 2020-05-07T00:48:08Z

I've looked into this because it might be a solution for #3158, at least for LDC - only emitting weak extern(C++) std::exception::what() etc. in druntime, so that they are ABI-compatible with the C++ ones and don't conflict with a potentially linked in C++ runtime.

tests/codegen/attr_weak.d

kinke · 2020-05-07T14:30:45Z

Is there any point in emitting Comdats for ELF? It prevents @weak from working properly (linker apparently prefers the version in the 1st object file, independent of whether it's weak or not), and clang doesn't emit them for Linux (well, for MSVC neither, but AFAIK, there the Comdats play the role of -f{function,data}-sections for linker stripping)...

JohanEngelen · 2020-05-07T17:46:37Z

tests/PGO/irbased_indirect_calls.d

@@ -27,7 +27,7 @@ extern (C)

    void function() foo;

-    @weak // disable reasoning about this function
+    pragma(inline, false)


This does not do the same thing. What we want is the function to be opaque. Disabling inlining still allows IPO to learn about what is written to foo. I think the same holds for optStrategy(none).
Edit: but applied to the hot function it does not matter here because there indeed we don't want inlining.

I just wanted to avoid additional complexity for string matching; for hot, disabling inlining wasn't enough, as the call was still elided. What worked was pragma(inline, false); llvm_sideeffect(); in the hot body, but optStrategy seemed nicer and had the same effect.

For select_func here, I thought disabling inlining would be enough (for string matching in the main body wrt. the foo function ptr); re-adding @weak wouldn't affect any strings though, and might even fix the regression with LLVM 4.0.

kinke · 2020-05-09T19:22:30Z

@rainers: Any suggestions wrt. how to 'best' adapt the mangled name (for backtrace/debugger/...)?

rainers · 2020-05-10T06:53:00Z

@rainers: Any suggestions wrt. how to 'best' adapt the mangled name (for backtrace/debugger/...)?

If you want it to be demangled with some modifications, prepending a package weak by changing the _D prefix to _D4weak should work.

Your use case in #3158 probably needs C++ symbols, though. VC mangling doesn't encode identifier length, so patching the first identifier might work, but it can be back referenced so it appears multiple times in the demangled name.

kinke · 2020-05-15T18:27:07Z

Cleaned up & put a little more effort into the renamed mangle.

clang emits COMDATs for templates and inline functions only (with linkonce_odr), not for regular functions, for both MSVC and Linux targets. I have no idea whether ELF COMDATs provide any benefit and so whether I should try harder and only get rid of them for regular functions, to make @weak work properly.

JohanEngelen · 2020-05-15T18:40:19Z

You could ask on the forums about the COMDATs on Linux, but I doubt you'll get useful feedback.
If Clang does not emit it, then I think it is fairly safe to omit them.

kinke · 2020-05-15T18:45:43Z

Yeah I wanted to know if you guys know more about this, otherwise I'd propose to include this in the upcoming beta and see how it goes.

kinke · 2020-05-15T20:00:36Z

Interestingly, the produced static libraries are now noticeably smaller for the Azure Linux x64 job (compared against latest master run):

libdruntime-ldc.a: 2,898 vs. 3,210 KB (-9.7%)
libphobos2-ldc.a: 9,466 vs. 10,393 KB (-8.9%)

The shared libs are minimally larger:

libdruntime-ldc-shared.so: 1,625 vs. 1,619 KB
libphobos2-ldc-shared.so: 6,202 vs. 6,173 KB (+0.5%)

kinke · 2020-05-15T20:37:28Z

gen/tollvm.cpp

-  if (lwc.second)
-    obj->setComdat(gIR->module.getOrInsertComdat(obj->getName()));
+  obj->setComdat(lwc.second ? gIR->module.getOrInsertComdat(obj->getName())
+                            : nullptr);


FWIW, I've also tried setting the COMDAT's selection kind to 'exact match' (instead of 'any'), but that only led back to the multiple definition errors.

@weak

…more Emulation for @weak global variables is still left out but should be analogous. The test adaptations are mostly a revert of 3893840. The testcase has shown that @weak hasn't worked properly for ELF (linker apparently prefers the version in the 1st object file, independent of whether it's weak or not), because the functions are emitted in COMDATs. clang emits COMDATs for templates and inline functions only, not for regular functions.

JohanEngelen · 2020-10-18T23:52:49Z

I just found out that the ELF COMDAT change is causing trouble at Weka. It prevents merging of symbols, leaving excess data around in a section where Weka's tooling wants there to be no garbage (or well, it just exceeds the amount of data that the system can deal with (ushort...)). Re-enabling COMDAT fixes it.
I guess we have to start doing what @kinke noted about clang (I think that would fix Weka's problem):

clang emits COMDATs for templates and inline functions only (with linkonce_odr), not for regular functions,

I'll work on a small testcase, so you can see what the problem is.

JohanEngelen · 2020-10-19T12:16:21Z

Opened an issue for it with testcase: #3589

JohanEngelen · 2020-10-22T21:20:23Z

LLVM 11 release notes:
"Windows target: Produce COFF weak external symbols for IR level weak symbols without a comdat (e.g. for attribute((weak)) in C)"
https://releases.llvm.org/11.0.0/docs/ReleaseNotes.html#changes-to-the-windows-target

@kinke Does that remove the need for the "emulation" of this PR ?

kinke · 2020-10-23T15:02:50Z

I've read that, found it a nice conincidence, but haven't tested whether it works similar to this manual emulation here. Anyway, it's not restricted to LLVM11+, and we always emit comdats on Windows.

kinke mentioned this pull request May 7, 2020

[WIP] core.stdcpp.{exception,typeinfo}: Fix D/C++ symbol conflicts and undefined D vtable entries dlang/druntime#2788

Closed

kinke commented May 7, 2020

View reviewed changes

tests/codegen/attr_weak.d Show resolved Hide resolved

kinke force-pushed the weak_msvc branch 2 times, most recently from 103a0ac to a5507cf Compare May 7, 2020 14:27

kinke force-pushed the weak_msvc branch from eb97bc8 to f774b15 Compare May 7, 2020 16:38

JohanEngelen reviewed May 7, 2020

View reviewed changes

kinke force-pushed the weak_msvc branch from f774b15 to 529e0fe Compare May 7, 2020 18:13

kinke force-pushed the weak_msvc branch from 529e0fe to 362eaeb Compare May 15, 2020 16:32

kinke changed the title ~~Windows: Add proper emulation for @weak functions~~ Add proper emulation for @weak functions and Windows targets, and don't emit COMDATs for ELF anymore May 15, 2020

kinke changed the title ~~Add proper emulation for @weak functions and Windows targets, and don't emit COMDATs for ELF anymore~~ Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore May 15, 2020

kinke force-pushed the weak_msvc branch from 362eaeb to 970df11 Compare May 15, 2020 18:20

kinke marked this pull request as ready for review May 15, 2020 18:20

kinke commented May 15, 2020

View reviewed changes

kinke force-pushed the weak_msvc branch from 970df11 to 9dac7c6 Compare May 16, 2020 18:43

druntime: Revise @weak usages

aaeb7f7

kinke force-pushed the weak_msvc branch from 9dac7c6 to aaeb7f7 Compare May 16, 2020 21:39

kinke merged commit 4eaa2fd into ldc-developers:master May 17, 2020

kinke deleted the weak_msvc branch May 17, 2020 10:58

ferencdg mentioned this pull request Sep 3, 2020

investigating why CircleCI failes with ldc-1.23 bosagora/agora#1167

Closed

JohanEngelen mentioned this pull request Oct 19, 2020

ELF COMDAT needed for templates to enable linker data culling #3589

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

kinke commented May 7, 2020

kinke commented May 7, 2020

kinke commented May 7, 2020 •

edited

Loading

JohanEngelen May 7, 2020 •

edited

Loading

kinke May 7, 2020

kinke commented May 9, 2020

rainers commented May 10, 2020

kinke commented May 15, 2020

JohanEngelen commented May 15, 2020

kinke commented May 15, 2020

kinke commented May 15, 2020

kinke May 15, 2020

JohanEngelen commented Oct 18, 2020

JohanEngelen commented Oct 19, 2020

JohanEngelen commented Oct 22, 2020

kinke commented Oct 23, 2020

Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

Conversation

kinke commented May 7, 2020

kinke commented May 7, 2020

kinke commented May 7, 2020 • edited Loading

JohanEngelen May 7, 2020 • edited Loading

Choose a reason for hiding this comment

kinke May 7, 2020

Choose a reason for hiding this comment

kinke commented May 9, 2020

rainers commented May 10, 2020

kinke commented May 15, 2020

JohanEngelen commented May 15, 2020

kinke commented May 15, 2020

kinke commented May 15, 2020

kinke May 15, 2020

Choose a reason for hiding this comment

JohanEngelen commented Oct 18, 2020

JohanEngelen commented Oct 19, 2020

JohanEngelen commented Oct 22, 2020

kinke commented Oct 23, 2020

kinke commented May 7, 2020 •

edited

Loading

JohanEngelen May 7, 2020 •

edited

Loading